test(skills): fail the build when a skill is not installable - #93
Merged
Conversation
Nine of thirteen skills were unreachable until 0.37.1 — shipped, published to npm, and listed by nobody, because /plugin marketplace add shows exactly the entries in .claude-plugin/marketplace.json. An entry got added alongside whichever skill prompted it and the rest were never backfilled. Nothing failed, because nothing looked. Guards both directions, since both are silent: a skills/<name>/SKILL.md with no entry is invisible to users, and an entry whose directory moved or was renamed is a broken install rather than a missing one. Also pins source == ./skills/ <name> (a mismatch installs the wrong skill under the right label, which looks like it worked), non-empty descriptions, and name uniqueness. Written as a test rather than a new script + CI step because npm test already gates PRs and publish, and because brand-numbers.test.ts established the pattern: assert the invariant against the real files in the repo that can fix it. Nothing here is auto-fixable anyway — a marketplace description is hand-written prose, and generating one from the frontmatter would produce worse text than the omission it replaces. Sibling to skill-frontmatter.test.ts, which proves a SKILL.md can LOAD. This proves it can be REACHED. Neither implies the other. Both take "a directory with SKILL.md at its root" as the definition of a skill — deliberately the same definition, or one of them can pass vacuously. Verified by breaking each invariant in turn and confirming the matching test fails: dropped entry, entry naming a nonexistent skill, wrong source path, empty description, duplicate name. The unregistered-skill failure names the directories and prints the JSON to paste. Test-only; skills/ and dist ship to npm but test/ does not, so this needs no version bump.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the recurrence risk left open by #91.
The gap
Nine of thirteen skills were unreachable until 0.37.1 — on disk, shipped inside the npm package, and installable by nobody, because
/plugin marketplace addshows exactly the entries in.claude-plugin/marketplace.json. #91 backfilled all thirteen but nothing stops the fourteenth from drifting the same way. The original cause was not a decision: an entry got added alongside whichever skill prompted it, and the rest were never backfilled. Nothing failed, because nothing looked.What this checks
Both directions, because both are silent:
skills/<name>/SKILL.mdhas an entryskills/<name>/SKILL.mdsource == ./skills/<name>descriptionWhy a test and not a script
npm testalready gates PRs andpublish.yml, so this needs no new CI step.brand-numbers.test.tsestablished the pattern in this repo: assert the invariant against the real files, in the repo that can fix it. And nothing here is usefully auto-fixable — a marketplace description is hand-written prose, so generating one from the frontmatter would produce worse text than the omission it replaces.It is the sibling of
skill-frontmatter.test.ts, which proves aSKILL.mdcan load. This proves it can be reached. Neither implies the other. Both take "a directory withSKILL.mdat its root" as the definition of a skill — the same definition on purpose, or one of them can pass vacuously.Verification
Green at 288/288, and confirmed it can actually fail — each invariant broken in turn, each producing the matching failure and nothing else:
The unregistered-skill failure names the directories and prints the JSON to paste:
Test-only —
skills/anddistship to npm,test/does not, so this needs no version bump or release.